Thanks for the PR. Would it be possible to add a regression test to e.g. tests/test_builtin.js? If you push the test first, we can do a before/after check; there are ASAN buildbots in the CI matrix.

@bnoordhuis thanks for taking a look!

I had added the test from the original issue bellard/quickjs#178. However it was still not working as expected. It turns out stack checking was disabled on this fork when using ASAN. I tried to re-enable it as an experiment so with make CONFIG_ASAN=ON && make test so it would pass locally. Not sure what best approach to take so thought I'd push what I had so far...

#161 - let's see if it's possible to get the stack checks working under asan

@nickva can you rebase on top of master?

Will do, thanks, @bnoordhuis!

@bnoordhuis Thank you for enabling ASAN stack checking.

I rebased on master and updated the test to try to account for the fact that without ASAN isArray might just work (return true), and with ASAN, it should raise an InternalError with a "stack overflow" message.

To validate it, I commented out the fix from quickjs.c and with ASAN enabled make test aborted:

  % make test
   ...
   #253 0x10e6b3550 in js_proxy_isArray+0x2c0 (qjs:x86_64+0x1000e9550)
   #254 0x10e6b3550 in js_proxy_isArray+0x2c0 (qjs:x86_64+0x1000e9550)

SUMMARY: AddressSanitizer: stack-overflow (qjs:x86_64+0x1000e9305) in js_proxy_isArray+0x75
==86352==ABORTING
make: *** [test] Abort trap: 6

With the fix, it passed the test.

I created a separate test PR which just has the test so we can see what it would do with the tests: #165

Cheers!

@bnoordhuis, @saghul thanks for starting and maintaining quickjs-ng!